Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[TF-PSA-Crypto] Move most of min_requirements.py to the framework #148

Merged
merged 2 commits into from
Jan 14, 2025
Merged

[TF-PSA-Crypto] Move most of min_requirements.py to the framework #148

merged 2 commits into from
Jan 14, 2025

Conversation

valeriosetti
Copy link
Contributor

@valeriosetti valeriosetti commented Jan 10, 2025 •
edited
Loading

As a part of solving Mbed-TLS/mbedtls-framework#86 and as requested here this PR copies min_requirements.py (the new version implemented in Mbed-TLS/mbedtls#9863) and *.requirements.txt files from the Mbed TLS repo to this one.

PR checklist

Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.

  • changelog provided | not required because:
  • framework PR provided Mbed-TLS/mbedtls-framework# | not required
  • mbedtls PR provided Mbed-TLS/mbedtls# | not required
  • tests provided | not required because:

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

Help make review efficient:

  • Multiple simple commits
    • please structure your PR into a series of small commits, each of which does one thing
  • Avoid force-push
    • please do not force-push to update your PR - just add new commit(s)
  • See our Guidelines for Contributors for more details about the review process.

This was referenced Jan 10, 2025
Merged
Merged
Open
@valeriosetti valeriosetti self-assigned this Jan 13, 2025
@valeriosetti valeriosetti added needs-review Every commit must be reviewed by at least two team members needs-reviewer This PR needs someone to pick it up for review priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most) labels Jan 13, 2025
ronald-cron-arm
ronald-cron-arm requested changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me but while at it please also add basic.requirements.txt and maintainer.requirements.txt.

@ronald-cron-arm ronald-cron-arm removed the needs-reviewer This PR needs someone to pick it up for review label Jan 13, 2025
@valeriosetti
scripts: copy min_requirements.py from the main repo
e73f2b3 
This commit also copies *.requirements.txt from the main repo.

Signed-off-by: Valerio Setti <[email protected]>
@valeriosetti
Copy link
Contributor Author

This looks good to me but while at it please also add basic.requirements.txt and maintainer.requirements.txt.

Ops, sorry, I didn't notice those files. It's done now ;)

ronald-cron-arm
ronald-cron-arm previously approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

Harry-Ramsey
Harry-Ramsey previously approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@Harry-Ramsey Harry-Ramsey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@valeriosetti
framework: update reference
8f227e3 
Signed-off-by: Valerio Setti <[email protected]>
@valeriosetti
Copy link
Contributor Author

If I remember correctly the only CI issue in full_without_ecdhe_ecdsa in test case Connection ID, 3D+MTU: Cli+Srv enabled, Cli+Srv CID nonempty is a "known failure glitch", so it should not be taken into account while evaluating this PR, which is therefore ready for reviews.

ronald-cron-arm
ronald-cron-arm approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Harry-Ramsey
Harry-Ramsey approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@Harry-Ramsey Harry-Ramsey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ronald-cron-arm ronald-cron-arm added this pull request to the merge queue Jan 14, 2025
Merged via the queue into Mbed-TLS:development with commit 21095c7 Jan 14, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Harry-Ramsey Harry-Ramsey Harry-Ramsey approved these changes

@ronald-cron-arm ronald-cron-arm ronald-cron-arm approved these changes

Assignees

@valeriosetti valeriosetti

Labels
needs-review Every commit must be reviewed by at least two team members priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most)
Projects
Roadmap pull requests (new board)
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@valeriosetti @ronald-cron-arm @Harry-Ramsey